Valkey #877
Valkey #877
Conversation
gateway-ha/src/main/java/io/trino/gateway/ha/router/BaseRoutingManager.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/main/java/io/trino/gateway/ha/router/BaseRoutingManager.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/main/java/io/trino/gateway/ha/router/DistributedCache.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/main/java/io/trino/gateway/ha/router/DistributedCache.java
Outdated
Show resolved
Hide resolved
- Fixed cacheTtlSeconds configuration not being used in ValkeyDistributedCache - Refactored repetitive distributedCache.isEnabled() checks into helper methods - Created QueryCacheManager to encapsulate cache management logic - Moved all cache classes to dedicated io.trino.gateway.ha.cache package - Renamed DistributedCache interface to Cache for better abstraction These changes provide better separation of concerns and make the caching infrastructure more maintainable and reusable across the gateway.
| @Singleton | ||
| public Cache getDistributedCache() | ||
| { | ||
| ValkeyConfiguration valkeyConfig = configuration.getValkeyConfiguration(); |
There was a problem hiding this comment.
Instead of referencing configuration directly, you should inject in HaGatewayConfiguration
|
|
||
| @Provides | ||
| @Singleton | ||
| public ValkeyConfiguration getValkeyConfiguration() |
There was a problem hiding this comment.
Inject in HaGatewayConfiguration here as well.
| private final long cacheTtlSeconds; | ||
|
|
||
| public ValkeyDistributedCache( | ||
| String host, |
There was a problem hiding this comment.
Instead of taking in all of these attributes, wouldn't it be beneficial to take in the ValkeyConfiguration object directly?
| return queryIdExternalUrlCache.get(queryId); | ||
| } | ||
|
|
||
| // L2 Cache Operations |
There was a problem hiding this comment.
Why does L1 vs L2 matter here? imo, this file shouldn't care about specific keys.
| this.queryIdBackendCache = buildCache(this::findBackendForUnknownQueryId); | ||
| this.queryIdRoutingGroupCache = buildCache(this::findRoutingGroupForUnknownQueryId); | ||
| this.queryIdExternalUrlCache = buildCache(this::findExternalUrlForUnknownQueryId); | ||
| this.queryCacheManager = new QueryCacheManager( |
There was a problem hiding this comment.
The QueryCacheManager should be injected into the constructor, not instantiated within the constructor. For dependency injection purposes.
| { | ||
| String backend; | ||
|
|
||
| // L2: Check Valkey distributed cache if enabled |
There was a problem hiding this comment.
The whole purpose of moving to the CacheManager is to not have to individually check L1 vs L2 vs ... you simply do a get and it'll return the value if it exists or not if it does not.
2 participants
##Add Valkey Distributed Cache for Horizontal Scaling
##Summary
This PR implements distributed caching using Valkey to enable horizontal scaling of Trino Gateway. Multiple gateway instances can now share query metadata through a distributed cache layer, ensuring consistent query routing across all instances.
##Motivation
Currently, Trino Gateway uses local Guava caches that are not shared between instances. In multi-instance deployments, this can lead to:
This implementation addresses these limitations while maintaining backward compatibility and graceful degradation.
##Architecture
3-Tier Caching Strategy
Request Flow:
- Hit: Return immediately
- Miss: Check L2
- Hit: Populate L1, return
- Miss: Check L3
- Found: Populate L2 + L1, return
- Not Found: Search backends via HTTP (~200ms)
Cache Keys
Three values are cached for each query:
All keys use configurable TTL (default 30 minutes / 1800 seconds).
##Implementation Details
Core Components
ValkeyConfiguration (gateway-ha/src/main/java/io/trino/gateway/ha/config/ValkeyConfiguration.java)
Cache Interface (gateway-ha/src/main/java/io/trino/gateway/ha/cache/Cache.java)
ValkeyDistributedCache (gateway-ha/src/main/java/io/trino/gateway/ha/cache/ValkeyDistributedCache.java)
QueryCacheManager (gateway-ha/src/main/java/io/trino/gateway/ha/cache/QueryCacheManager.java) - NEW
NoopDistributedCache (gateway-ha/src/test/java/io/trino/gateway/ha/cache/NoopDistributedCache.java)
Integration
BaseRoutingManager - Simplified routing logic:
ProxyRequestHandler - Query submission:
HaGatewayProviderModule - Dependency injection:
Configuration
Minimal (Recommended for Getting Started)
With Authentication
Advanced (Production Tuning)
Single Instance (No Changes Required)
##Testing
Unit Tests
TestValkeyConfiguration
TestValkeyDistributedCache (2 tests)
Integration Tests
TestValkeyDistributedCacheIntegration (9 comprehensive tests using TestContainers)
TestRoutingManagerExternalUrlCache (6 tests)
TestContainers Setup
Test Results
##Backward Compatibility
✅ Fully backward compatible
Migration Path
From Single to Multi-Gateway:
docker run -d -p 6379:6379 valkey/valkey:latest
valkeyConfiguration:
enabled: true
host: valkey.internal
port: 6379
password: ${VALKEY_PASSWORD}
Check Valkey keys
docker exec valkey valkey-cli KEYS "trino:query:*"
No data migration needed - cache populates automatically.
##Graceful Degradation
When Valkey is unavailable:
Dependencies
Added:
###Code Quality Improvements
New Files (8)
Core Implementation:
Tests:
Modified Files (10)
Configuration:
Core:
Build:
Tests:
Future Enhancements